Closed
Bug 1402376
Opened 8 years ago
Closed 8 years ago
Add JAWS client information to update url
Categories
(Toolkit :: Application Update, enhancement)
Tracking
()
VERIFIED
FIXED
mozilla58
People
(Reporter: jimm, Assigned: robert.strong.bugs)
References
(Blocks 1 open bug)
Details
(Whiteboard: aes+)
Attachments
(2 files, 2 obsolete files)
1.28 KB,
patch
|
jimm
:
review+
Sylvestre
:
approval-mozilla-beta+
Sylvestre
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
1.70 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
We have compatibility issues with 57 w/e10s and the current release of the JAWS accessibility client. A software update from the vendor is forthcoming.
However we can't expect old clients to receive updates and we've exhausted methods for fixing older compat issues. So we want to add the following work around:
1) add the current jaws injection dll version to our updater url
2) use balrog to avoid sending Firefox 57 updates to these users
We'll need to get this change into 56, which is currently at rc2. So this is really pressing.
![]() |
Reporter | |
Updated•8 years ago
|
Assignee: nobody → jmathies
![]() |
Reporter | |
Updated•8 years ago
|
Version: 47 Branch → 57 Branch
![]() |
Reporter | |
Comment 1•8 years ago
|
||
Looks like we were using the aus helper for this, in which case we may want to use that again and ship an update in 56.
http://searchfox.org/mozilla-central/source/browser/extensions/aushelper/bootstrap.js#150
![]() |
Reporter | |
Comment 2•8 years ago
|
||
I'm not sure which direction to take here, I could do this in front end js as well, which might be faster. Robert, any suggestions here?
Flags: needinfo?(robert.strong.bugs)
![]() |
Reporter | |
Comment 3•8 years ago
|
||
Comment 5•8 years ago
|
||
If this can land today, I can still get it into the RC4 build. For now, I'll mark it as a 56 release blocker.
Or, is this something we can plan to go into a 56 dot release?
![]() |
Reporter | |
Comment 6•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5)
> If this can land today, I can still get it into the RC4 build. For now, I'll
> mark it as a 56 release blocker.
>
> Or, is this something we can plan to go into a 56 dot release?
Either works, whichever causes fewer hassles. Just need to get this to 56 users before 57 ships in November.
![]() |
Assignee | |
Comment 7•8 years ago
|
||
Since there will be a requirement to update to 56 let's go with 56. This should add the value to SYSTEM_CAPABILITIES.
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm#87
Specifically, let's go with JAWS: 0 for when JAWS is not detected and JAWS: 1 for when it is detected.
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm#138
Flags: needinfo?(robert.strong.bugs)
![]() |
Assignee | |
Comment 8•8 years ago
|
||
Meant to say JAWS:0 and JAWS:1 (e.g. no space)
Comment 9•8 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1402411 for the Balrog bits needed to support matching on this -- but they don't block the client changes.
Flags: needinfo?(bhearsum)
See Also: → 1402411
![]() |
Assignee | |
Comment 10•8 years ago
|
||
Client code only... still need to check the tests
Assignee: jmathies → robert.strong.bugs
Attachment #8911248 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8911271 -
Flags: review?(jmathies)
![]() |
Assignee | |
Comment 11•8 years ago
|
||
Test changes coming up
Attachment #8911271 -
Attachment is obsolete: true
Attachment #8911271 -
Flags: review?(jmathies)
Attachment #8911279 -
Flags: review?(jmathies)
![]() |
Reporter | |
Updated•8 years ago
|
Attachment #8911279 -
Flags: review?(jmathies) → review+
![]() |
Assignee | |
Comment 12•8 years ago
|
||
Attachment #8911287 -
Flags: review?(jmathies)
![]() |
Assignee | |
Comment 13•8 years ago
|
||
Example url generated from my local Windows Firefox 64 bit build
https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922112255/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%2010.0.0.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32620,JAWS:0/default/default/update.xml?force=1
![]() |
Reporter | |
Updated•8 years ago
|
Attachment #8911287 -
Flags: review?(jmathies) → review+
![]() |
Reporter | |
Comment 14•8 years ago
|
||
I just finished up preliminary testing of this:
STR #1
1) install the 17 or current release version 18 of the JAWS screen reader.
2) Run JAWS
3) launch Firefox 56 or 57 with this patch and do an update check
AUS:SVC Checker:checkForUpdates - sending request to: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/default/update.xml?force=1
STR #2:
1) install an internal build of the JAWS screen reader (See dbolter, grover, or myself for access)
2) Run JAWS
3) launch Firefox 56 or 57 with this patch and do an update check
AUS:SVC Checker:checkForUpdates - sending request to: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/default/update.xml?force=1
STR #3:
1) do not install jaws
2) launch Firefox 56 or 57 with this patch and do an update check
AUS:SVC Checker:getUpdateURL - update URL: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/default/update.xml?force=1
![]() |
Assignee | |
Comment 15•8 years ago
|
||
Comment on attachment 8911279 [details] [diff] [review]
patch rev2
Approval Request Comment
[Feature/Bug causing the regression]: not sure... ask jimm
[User impact if declined]: clients with an incompatible jaws client will get updated to a Firefox version that is incompatible with the jaws client
[Is this code covered by automated tests?]: yes. This code is a consumer of nsIXULRuntime shouldBlockIncompatJaws which has been around for awhile and the tests don't cover that code.
[Has the fix been verified in Nightly?]: No. It has been manually verified though.
[Needs manual test from QE? If yes, steps to reproduce]: See comment #14
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is checking the value of nsIXULRuntime shouldBlockIncompatJaws which has been around awhile and based on that adding a string to the update url.
[String changes made/needed]: None
Attachment #8911279 -
Flags: approval-mozilla-beta?
![]() |
Assignee | |
Comment 16•8 years ago
|
||
Comment on attachment 8911279 [details] [diff] [review]
patch rev2
forgot that this needs to also go on release
Approval Request Comment
[Feature/Bug causing the regression]: not sure... ask jimm
[User impact if declined]: clients with an incompatible jaws client will get updated to a Firefox version that is incompatible with the jaws client
[Is this code covered by automated tests?]: yes. This code is a consumer of nsIXULRuntime shouldBlockIncompatJaws which has been around for awhile and the tests don't cover that code.
[Has the fix been verified in Nightly?]: No. It has been manually verified though.
[Needs manual test from QE? If yes, steps to reproduce]: See comment #14
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is checking the value of nsIXULRuntime shouldBlockIncompatJaws which has been around awhile and based on that adding a string to the update url.
[String changes made/needed]: None
Attachment #8911279 -
Flags: approval-mozilla-release?
Comment 17•8 years ago
|
||
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2d9582ceb1f
client code - Add whether the client has an incompatible version of JAWS to the update url. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/3870c3b2cb54
test code - Add whether the client has an incompatible version of JAWS to the update url. r=jimm
![]() |
Reporter | |
Comment 18•8 years ago
|
||
Note, nsIXULRuntime's shouldBlockIncompatJaws landed in 56.
Comment 19•8 years ago
|
||
Comment on attachment 8911279 [details] [diff] [review]
patch rev2
JAWS accessibility support, taking it.
Should be in the next 56 rc + 57b3
Attachment #8911279 -
Flags: approval-mozilla-release?
Attachment #8911279 -
Flags: approval-mozilla-release+
Attachment #8911279 -
Flags: approval-mozilla-beta?
Attachment #8911279 -
Flags: approval-mozilla-beta+
Comment 20•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a5cb7f20d1e3
https://hg.mozilla.org/releases/mozilla-beta/rev/0a202bb2e249
status-firefox57:
--- → fixed
Flags: in-testsuite+
Comment 21•8 years ago
|
||
bugherder uplift |
![]() |
Assignee | |
Comment 22•8 years ago
|
||
Henrik, just a heads up. I don't think this will affect your tests.
Flags: needinfo?(hskupin)
![]() |
||
Comment 23•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a2d9582ceb1f
https://hg.mozilla.org/mozilla-central/rev/3870c3b2cb54
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 24•8 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #14)
> STR #1
> 1) install the 17 or current release version 18 of the JAWS screen reader.
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
>
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> default/update.xml?force=1
Can this be verified with https://archive.mozilla.org/pub/firefox/candidates/56.0-candidates/build4/win64/en-US/Firefox%20Setup%2056.0.exe ? The dxr results for release look a little sparse - https://dxr.mozilla.org/mozilla-release/search?q=shouldBlockIncompatJaws
![]() |
Assignee | |
Comment 25•8 years ago
|
||
See comment #18
![]() |
Assignee | |
Comment 26•8 years ago
|
||
Jim, can you verify that shouldBlockIncompatJaws is available per comment #24?
Flags: needinfo?(jmathies)
Comment 27•8 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #22)
> Henrik, just a heads up. I don't think this will affect your tests.
Yeah, shouldn't make a difference. But I don't maintain those tests anymore.
Flags: needinfo?(hskupin)
Comment 28•8 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #26)
> Jim, can you verify that shouldBlockIncompatJaws is available per comment
> #24?
As far as I can tell it was added in bug 1385991, which isn't in 56.
![]() |
Assignee | |
Comment 29•8 years ago
|
||
Jim, if the code to support incompatible JAWS didn't make it to 56 could you please get it uplifted? It is very important that this is added to 56 and not a point release so we don't have to create multiple watersheds.
Liz, heads up.
Flags: needinfo?(lhenry)
![]() |
Assignee | |
Comment 30•8 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #29)
> Jim, if the code to support incompatible JAWS didn't make it to 56 could you
s/incompatible JAWS/incompatible JAWS detection/
Comment 31•8 years ago
|
||
Rob, does that prevent us to ship rc4 to the beta population or not?
Any potential bad side effect?
Flags: needinfo?(robert.strong.bugs)
Comment 32•8 years ago
|
||
As far as I can tell it'll just never set JAWS:1 in the update URL because Services.appinfo.shouldBlockIncompatJaws is 'undefined'.
![]() |
Assignee | |
Comment 33•8 years ago
|
||
I am quite certain comment #32 is the case.
It looks like Jim will need to uplift at least part of bug 1385991 to be able to detect this before updating clients to 57 and it would be a good thing to do this in Firefox 56.0 so we don't have to do one of the following items if the portion of bug 1385991 that is needed is uplifted to a 56.0.x release
1. releng could create additional updates for pre Firefox 56.0 Windows clients to update to the 56.0.x release with the detection. This is additional work for releng across all locales and additional complexity for the balrog rules.
2. if the additional work by releng is not performed then Windows clients would have to update to 56.0 and then update to the 56.0.x release that contains the detection before updating to latest. Essentially a funnel and funnels take longer by nature which affects orphaning especially for clients that seldom run Firefox.
Flags: needinfo?(robert.strong.bugs)
Comment 34•8 years ago
|
||
OK, so, Liz will make the call but I propose that we still push 56rc4 to the beta channel, we build a RC5 and we do extra testing on this bug.
Sounds good?
Comment 35•8 years ago
|
||
Agreed, I'd like to push RC4, looking now to see if QE has signed off.
Flags: needinfo?(lhenry)
Comment 36•8 years ago
|
||
Jimm, can you request uplift on whatever parts of bug 1385991 and its dependency(s) should get onto 56 before we release 56? Thanks.
![]() |
Reporter | |
Comment 37•8 years ago
|
||
I had searched across what I though was 56 and found shouldBlockIncompatJaws last week, but apparently wasn't looking at the 56 code base.
Flags: needinfo?(jmathies)
Comment 38•8 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #14)
> I just finished up preliminary testing of this:
>
> STR #1
> 1) install the 17 or current release version 18 of the JAWS screen reader.
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
>
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> default/update.xml?force=1
>
> STR #2:
> 1) install an internal build of the JAWS screen reader (See dbolter, grover,
> or myself for access)
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
>
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> default/update.xml?force=1
>
> STR #3:
> 1) do not install jaws
> 2) launch Firefox 56 or 57 with this patch and do an update check
>
> AUS:SVC Checker:getUpdateURL - update URL:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> default/update.xml?force=1
Hi Jim, a few questions here:
• The links provided in the URL should be the value of app.update.url in about:config?
• I was asked to test with this treeherder build located here: https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&revision=8c340d0042b510bf984437ca3e5bbbe06daf95d4&selectedJob=133164266 but it seems that the only working build is that of Windows 2012 addon opt. Is that acceptable to test? The Windows 2012 opt is listed as tc(N) and I am unable to open that.
Flags: needinfo?(jmathies)
![]() |
Reporter | |
Updated•8 years ago
|
Flags: needinfo?(jmathies)
![]() |
Reporter | |
Comment 39•8 years ago
|
||
(In reply to Grover Wimberly IV [:Grover-QA] from comment #38)
> (In reply to Jim Mathies [:jimm] from comment #14)
> > I just finished up preliminary testing of this:
> >
> > STR #1
> > 1) install the 17 or current release version 18 of the JAWS screen reader.
> > 2) Run JAWS
> > 3) launch Firefox 56 or 57 with this patch and do an update check
> >
> > AUS:SVC Checker:checkForUpdates - sending request to:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> > default/update.xml?force=1
> >
> > STR #2:
> > 1) install an internal build of the JAWS screen reader (See dbolter, grover,
> > or myself for access)
> > 2) Run JAWS
> > 3) launch Firefox 56 or 57 with this patch and do an update check
> >
> > AUS:SVC Checker:checkForUpdates - sending request to:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> > default/update.xml?force=1
> >
> > STR #3:
> > 1) do not install jaws
> > 2) launch Firefox 56 or 57 with this patch and do an update check
> >
> > AUS:SVC Checker:getUpdateURL - update URL:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> > default/update.xml?force=1
>
> Hi Jim, a few questions here:
>
> • The links provided in the URL should be the value of app.update.url in
> about:config?
> • I was asked to test with this treeherder build located here:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> release&revision=8c340d0042b510bf984437ca3e5bbbe06daf95d4&selectedJob=1331642
> 66 but it seems that the only working build is that of Windows 2012 addon
> opt. Is that acceptable to test? The Windows 2012 opt is listed as tc(N) and
> I am unable to open that.
Grover and I worked this out over IRC.
Comment 40•8 years ago
|
||
Tested the three scenarios with the treeherder 56 build given earlier today, values for JAWS and the URLs match all except the version number from Comment 14 (56.0 as opposed to 58.0a1). Marking as Verified. If there are any further issues, please NI me. Thanks.
You need to log in
before you can comment on or make changes to this bug.
Description
•